Conversation
| #[cfg(not(feature = "zcash"))] | ||
| pub fn required_balance_for_execute_refund(&self) -> NearToken { | ||
| // execute_refund uses ~700 bytes of storage (BTCPendingInfo + Account + verified_deposit_utxo) | ||
| // At 0.00001 NEAR/byte, that's ~0.007 NEAR. We use 0.1 NEAR as a safe margin. |
| /// | ||
| /// # Arguments | ||
| /// | ||
| /// * `deposit_msg` - The original deposit message (must contain `refund_address`). |
There was a problem hiding this comment.
This doesn't match function definition where refund_address is separate
| ); | ||
|
|
||
| // Must not be already verified/finalized | ||
| require!( |
There was a problem hiding this comment.
Why don't we only check it in the callback?
There was a problem hiding this comment.
To avoid unnecessary triggering of the check in btc_light_client. In general, it can be done only in the callback.
| #[cfg(not(feature = "zcash"))] | ||
| #[payable] | ||
| #[access_control_any(roles(Role::DAO))] | ||
| pub fn set_refund_timelock_sec(&mut self, refund_timelock_sec: u64) { |
There was a problem hiding this comment.
The number of setters has become huge, can we combine some of them under one setter, eg set_config?
| vout: usize, | ||
| tx_block_blockhash: String, | ||
| tx_index: u64, | ||
| merkle_proof: Vec<String>, |
There was a problem hiding this comment.
are we going to add coinbase proof here?
I would like. to refactor it and create specific struct for the proof arguments so it can be reused between methods.
Also please verify the performance of this call, maybe we can optimize it by using borsh, or at least make the tx_bytes readable by using hex or baseXX
There was a problem hiding this comment.
I would do this in the PR where the coinbase proof is added. I’m not sure I’ll add the coinbase proof everywhere, because some relayers might be difficult to migrate. For example, those that do active UTXO management.
|
|
||
| // Must not have a pending refund request already | ||
| require!( | ||
| !self.data().refund_requests.contains_key(&utxo_storage_key), |
There was a problem hiding this comment.
I think we can move all these checks to the callback, the sdk should be smart enough to not call this method if it could fails
| state.stage = PendingInfoStage::PendingBurn; | ||
| } | ||
| PendingInfoState::Refund(state) => { | ||
| state.stage = PendingInfoStage::PendingBurn; |
There was a problem hiding this comment.
It might be a misunderstanding from my side, but I think refund never triggers burn action, so marking it as PendingBurn is invalid state, no?
There was a problem hiding this comment.
On one hand, you’re right, but I left this stage for consistency with all the other pipelines. Like sign, that it is the final stage.
There was a problem hiding this comment.
Maybe we can use panic or unreachable here to make it more explicit? Or there might be a case when we enter this branch?
4 participants
Added a refund mechanism for cases when the relayer does not want to finalize a deposit transaction for any reason.
A user creates a refund request, and if the deposit is not finalized within a certain time period, execute_refund can be called to return the funds back to Bitcoin. DAO/Operator can reject the proposal.